IGNITE-27088 BinaryWriter should use internal String#value by nizhikov · Pull Request #13529 · apache/ignite

nizhikov · 2026-08-27T14:57:54Z

Thank you for submitting the pull request to the Apache Ignite.

In order to streamline the review of the contribution
we ask you to ensure the following steps have been taken:

The Contribution Checklist

There is a single JIRA ticket related to the pull request.
The web-link to the pull request is attached to the JIRA ticket.
The JIRA ticket has the Patch Available state.
The pull request body describes changes that have been made.
The description explains WHAT and WHY was made instead of HOW.
The pull request title is treated as the final commit message.
The following pattern must be used: IGNITE-XXXX Change summary where XXXX - number of JIRA issue.
A reviewer has been mentioned through the JIRA comments
(see the Maintainers list)
The pull request has been checked by the Teamcity Bot and
the green visa attached to the JIRA ticket (see tab PR Check at TC.Bot - Instance 1 or TC.Bot - Instance 2)

Notes

If you need any help, please email dev@ignite.apache.org or ask anу advice on http://asf.slack.com #ignite channel.

ignitetcbot · 2026-09-04T15:54:55Z

TCBot Test Analysis

TeamCity: --> Run :: All Results
Branch: pull/13529/head
Base: master
Tested commit: 75b0200

Possible Blockers (0)

No blockers found.

New Tests (4)

Binary Objects: 4 tests
- IgniteBinaryObjectsTestSuite: StringWriterSelfTest.testCorpus - PASSED
- IgniteBinaryObjectsTestSuite: StringWriterSelfTest.testLargeStrings - PASSED
- IgniteBinaryObjectsTestSuite: StringWriterSelfTest.testRandomStrings - PASSED
- IgniteBinaryObjectsTestSuite: StringWriterSelfTest.testStreamPosition - PASSED

Copilot

🟡 Changes recommended

A Checkstyle-enforced unused import in StringWriter.java will break the build, and the JMH benchmark’s zeroCopy parameterization is not reliable due to JVM-static initialization of the toggle.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Introduces a zero-copy UTF-8 string serialization path for Ignite binary writing, controlled by a new system property, and adds tests/benchmarks to validate and measure the behavior.

Changes:

Added StringWriter to serialize String to BinaryOutputStream without allocating temporary UTF-8 byte arrays when enabled.
Added IGNITE_BINARY_STRING_ZERO_COPY system property (default true) and wired it into BinaryWriterExImpl.
Updated DirectByteBufferStream string encoding/decoding to be explicitly UTF-8 and added coverage/benchmarking for the new string writer.

File summaries

File	Description
modules/core/src/test/java/org/apache/ignite/testsuites/IgniteBinaryObjectsTestSuite.java	Adds the new `StringWriterSelfTest` to the binary objects test suite.
modules/core/src/test/java/org/apache/ignite/internal/binary/StringWriterSelfTest.java	New differential tests ensuring `StringWriter` output matches UTF-8 `String#getBytes` serialization.
modules/core/src/main/java/org/apache/ignite/internal/direct/stream/DirectByteBufferStream.java	Uses UTF-8 explicitly for string serialization; adds ASCII fast-path via internal string value.
modules/commons/src/main/java/org/apache/ignite/IgniteCommonsSystemProperties.java	Adds `IGNITE_BINARY_STRING_ZERO_COPY` property and default value constant.
modules/binary/impl/src/main/java/org/apache/ignite/internal/binary/StringWriter.java	New zero-copy UTF-8 string writer with compact-string and SIMD-negative-scan optimizations when available.
modules/binary/impl/src/main/java/org/apache/ignite/internal/binary/BinaryWriterExImpl.java	Enables `StringWriter` path when `IGNITE_BINARY_STRING_ZERO_COPY` is enabled.
modules/benchmarks/src/main/java/org/apache/ignite/internal/benchmarks/jmh/binary/JmhBinaryStringWriteBenchmark.java	Adds JMH benchmark intended to compare zero-copy vs legacy behavior.

Review details

Files reviewed: 7/7 changed files
Comments generated: 3
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+/**
+ * Tests that {@link StringWriter} output is byte-identical to serialization of the {@link String#getBytes()} result,
+ * which was used before zero-copy string serialization was introduced.
+ */


zstan · 2026-09-07T12:39:59Z

+        byte[] latin1 = latin1Value(val);
+
+        if (latin1 != null) {
+            if (out.hasArray()) {


.NET/C++ code goes through hasArray()==false branch as I can see, thus we need carefully bench such a branch too.

zstan · 2026-09-07T13:01:21Z

+                Field valField = String.class.getDeclaredField("value");
+                Field coderField = String.class.getDeclaredField("coder");
+
+                // On JDK 8 the value field is a char[], only the generic encoder can be used.


JDK 8 can not be reached cause currently target (source/target 17), thus this message and (probably check) is redundant

zstan · 2026-09-07T13:43:49Z

+
+        str = sb.toString();
+
+        out = BinaryStreams.outputStream(4 * len + 64);


I try to run it instead of master with fast refactoring for PlatformOutputStreamImpl (C++.Net) like :
and found a bit perf drop (in measurements !!!!) thus real performance test need to be executed.
I changed:

long poolPtr = allocatePool(); long memPtr = allocatePooled(poolPtr, 10_000); PlatformMemory res = pool.get(memPtr); out = res.output(); //out = new PlatformAbstractMemory BinaryStreams.outputStream(4 * len + 64);

PR Benchmark (content) (len) (zeroCopy) Mode Cnt Score Error Units JmhBinaryStringWriteBenchmark.writeString cyrillic 512 true avgt 5 523.005 ± 13.730 ns/op JmhBinaryStringWriteBenchmark.writeString:gc.alloc.rate cyrillic 512 true avgt 5 0.001 ± 0.001 MB/sec master Benchmark (content) (len) (zeroCopy) Mode Cnt Score Error Units JmhBinaryStringWriteBenchmark.writeString cyrillic 512 true avgt 5 443.634 ± 11.679 ns/op JmhBinaryStringWriteBenchmark.writeString:gc.alloc.rate cyrillic 512 true avgt 5 5572.091 ± 145.617 MB/sec

nizhikov added 7 commits September 4, 2026 16:55

IGNITE-27088 BinaryWriter should use internal String#value

2cea055

IGNITE-27088 BinaryWriter should use internal String#value

b6feacc

IGNITE-27088 BinaryWriter should use internal String#value

9394099

IGNITE-27088 BinaryWriter should use internal String#value

cbb0052

IGNITE-27088 BinaryWriter should use internal String#value

d36ef1a

IGNITE-27088 BinaryWriter should use internal String#value

55feffd

IGNITE-27088 BinaryWriter should use internal String#value

ccb6f14

nizhikov force-pushed the IGNITE-27088 branch from 75b0200 to ccb6f14 Compare September 4, 2026 14:36

nizhikov added 2 commits September 4, 2026 17:53

IGNITE-27088 BinaryWriter should use internal String#value

351fd2b

IGNITE-27088 BinaryWriter should use internal String#value

f7ce522

zstan requested a lite review from Copilot September 7, 2026 06:14

Copilot started reviewing on behalf of zstan September 7, 2026 06:14 View session

Copilot AI reviewed Sep 7, 2026

View reviewed changes

zstan reviewed Sep 7, 2026

View reviewed changes

nizhikov added 2 commits September 7, 2026 14:46

Merge branch 'master' into IGNITE-27088

5533d45

IGNITE-27088 BinaryWriter should use internal String#value

fcdf587

zstan reviewed Sep 7, 2026

View reviewed changes

zstan approved these changes Sep 7, 2026

View reviewed changes

Navigation Menu

Uh oh!

IGNITE-27088 BinaryWriter should use internal String#value - #13529

IGNITE-27088 BinaryWriter should use internal String#value#13529
nizhikov wants to merge 11 commits into
apache:masterfrom
nizhikov:IGNITE-27088

nizhikov commented Aug 27, 2026

Uh oh!

ignitetcbot commented Sep 4, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zstan Sep 7, 2026

Uh oh!

zstan Sep 7, 2026

Uh oh!

Uh oh!

zstan Sep 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants


		str = sb.toString();

		out = BinaryStreams.outputStream(4 * len + 64);

Uh oh!

Conversation

nizhikov commented Aug 27, 2026

The Contribution Checklist

Notes

Uh oh!

ignitetcbot commented Sep 4, 2026

TCBot Test Analysis

Possible Blockers (0)

New Tests (4)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zstan Sep 7, 2026

Choose a reason for hiding this comment

Uh oh!

zstan Sep 7, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zstan Sep 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants